Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aa): refactor Automated Analysis Recording Setting #1014

Merged
merged 10 commits into from
May 5, 2023

Conversation

maxcao13
Copy link
Member

@maxcao13 maxcao13 commented May 3, 2023

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed the last commit: git commit --amend --signoff

Fixes: #1006

Description of the change:

This puts the current configuration as a expandable panel in the AA recording config settings and allows the user to explicitly edit the configuration. It also prevents null templates from being set as a default configuration.

Motivation for the change:

See #1006

@maxcao13 maxcao13 added chore Refactor, rename, cleanup, etc. backport labels May 3, 2023
@maxcao13 maxcao13 requested review from andrewazores and tthvo May 3, 2023 21:45
@mergify mergify bot added the safe-to-test label May 3, 2023
@github-actions
Copy link

github-actions bot commented May 3, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1014-bcdf3827fe85293d13a97d66aab8da160adc453f sh smoketest.sh

@github-actions
Copy link

github-actions bot commented May 3, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1014-c26de9f5e025edaba5e5ac44d4a9506c83a24c5b sh smoketest.sh

@tthvo tthvo marked this pull request as draft May 3, 2023 23:30
@github-actions
Copy link

github-actions bot commented May 4, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1014-9177d5e7799fa324abae16f0dff2af7a47151bfe sh smoketest.sh

@maxcao13 maxcao13 marked this pull request as ready for review May 4, 2023 08:19
@github-actions
Copy link

github-actions bot commented May 4, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1014-b7b73fe892f929f0a71669b3a62d0e628d4741c0 sh smoketest.sh

Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI looks good!

Not sure if its just me. The editing panel seems crashing in Automated Analysis Drawer -> click on the edit icon just closes the drawer panel.

I think it is the issue with TargetSelect trying to set global selection. This causes the AutomatedAnalysisCard to generateReport -> loading state -> closes the drawer.

See #1014 (comment)

locales/en/public_old.json Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented May 4, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1014-89239a745c9117972e0c45fad467de87747f5b45 sh smoketest.sh

Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more comments and prettier needs running :))

maxcao13 added 4 commits May 4, 2023 17:18
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
maxcao13 added 5 commits May 4, 2023 17:18
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
Signed-off-by: Max Cao <macao@redhat.com>
@github-actions
Copy link

github-actions bot commented May 5, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1014-fb3ded1071be0d1ba21e47869ae5b40e18b2a38a sh smoketest.sh

tthvo
tthvo previously approved these changes May 5, 2023
Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! Thanks!

@maxcao13
Copy link
Member Author

maxcao13 commented May 5, 2023

@andrewazores any other comments?

Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good other than the two minor localization spots

Signed-off-by: Max Cao <macao@redhat.com>
@github-actions
Copy link

github-actions bot commented May 5, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1014-40d2ba7a27286d3c1f21486a9150f39677474313 sh smoketest.sh

@ebaron ebaron merged commit 093e003 into cryostatio:main May 5, 2023
mergify bot pushed a commit that referenced this pull request May 5, 2023
* show current config

Signed-off-by: Max Cao <macao@redhat.com>

* prevent setting empty template

Signed-off-by: Max Cao <macao@redhat.com>

* add reset

Signed-off-by: Max Cao <macao@redhat.com>

* fix some handling

Signed-off-by: Max Cao <macao@redhat.com>

* redesign

Signed-off-by: Max Cao <macao@redhat.com>

* redesign 2

Signed-off-by: Max Cao <macao@redhat.com>

* update tests and add confirmation

Signed-off-by: Max Cao <macao@redhat.com>

* re: fixes

Signed-off-by: Max Cao <macao@redhat.com>

* fix tests

Signed-off-by: Max Cao <macao@redhat.com>

* localize

Signed-off-by: Max Cao <macao@redhat.com>

---------

Signed-off-by: Max Cao <macao@redhat.com>
(cherry picked from commit 093e003)
ebaron pushed a commit that referenced this pull request May 5, 2023
* show current config

Signed-off-by: Max Cao <macao@redhat.com>

* prevent setting empty template

Signed-off-by: Max Cao <macao@redhat.com>

* add reset

Signed-off-by: Max Cao <macao@redhat.com>

* fix some handling

Signed-off-by: Max Cao <macao@redhat.com>

* redesign

Signed-off-by: Max Cao <macao@redhat.com>

* redesign 2

Signed-off-by: Max Cao <macao@redhat.com>

* update tests and add confirmation

Signed-off-by: Max Cao <macao@redhat.com>

* re: fixes

Signed-off-by: Max Cao <macao@redhat.com>

* fix tests

Signed-off-by: Max Cao <macao@redhat.com>

* localize

Signed-off-by: Max Cao <macao@redhat.com>

---------

Signed-off-by: Max Cao <macao@redhat.com>
(cherry picked from commit 093e003)

Co-authored-by: Max Cao <macao@redhat.com>
@maxcao13 maxcao13 deleted the aa-settings branch May 5, 2023 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport chore Refactor, rename, cleanup, etc. safe-to-test
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Task] Redesign Automated Analysis Recording Setting
4 participants